Sync release-3.8.3 with vb/trace: role grant - #155

src/main/java/com/iemr/tm/controller/teleconsultation/TeleConsultationController.java (1)

38-40: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Remove unused imports and injected fields across controllers.

Both TeleConsultationController and VideoConsultationController declare imports for CookieUtil, JwtUtil (and HttpServletRequest) and inject a JwtUtil bean. Since the endpoint authentication logic has been refactored to rely exclusively on the Authentication principal passed as a method parameter, these utilities are no longer used.

• src/main/java/com/iemr/tm/controller/teleconsultation/TeleConsultationController.java#L38-L40: Remove the CookieUtil and JwtUtil imports.
• src/main/java/com/iemr/tm/controller/teleconsultation/TeleConsultationController.java#L58-L60: Remove the unused @Autowired private JwtUtil jwtUtil; field.
• src/main/java/com/iemr/tm/controller/videoconsultationcontroller/VideoConsultationController.java#L37-L40: Remove the HttpServletRequest, CookieUtil, and JwtUtil imports.
• src/main/java/com/iemr/tm/controller/videoconsultationcontroller/VideoConsultationController.java#L53-L54: Remove the unused @Autowired private JwtUtil jwtUtil; field.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@src/main/java/com/iemr/tm/controller/teleconsultation/TeleConsultationController.java`
around lines 38 - 40, Remove the unused CookieUtil and JwtUtil imports and
injected JwtUtil field from TeleConsultationController at
src/main/java/com/iemr/tm/controller/teleconsultation/TeleConsultationController.java
lines 38-40 and 58-60. In VideoConsultationController, remove the unused
HttpServletRequest, CookieUtil, and JwtUtil imports plus the injected JwtUtil
field at
src/main/java/com/iemr/tm/controller/videoconsultationcontroller/VideoConsultationController.java
lines 37-40 and 53-54; keep authentication based on the Authentication method
parameter.

src/main/java/com/iemr/tm/controller/cancerscreening/CancerScreeningController.java (1)

143-143: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Simplify @PreAuthorize role checks with hasAnyRole.

Using Spring Security's native hasAnyRole('ROLE1', 'ROLE2') provides a more concise and idiomatic expression than chaining multiple hasRole(...) || hasRole(...) blocks. Consider updating the authorization logic across the controller endpoints to improve readability.

• src/main/java/com/iemr/tm/controller/cancerscreening/CancerScreeningController.java#L143-L143: use hasAnyRole('NURSE', 'DOCTOR').
• src/main/java/com/iemr/tm/controller/cancerscreening/CancerScreeningController.java#L173-L173: use hasAnyRole('NURSE', 'DOCTOR').
• src/main/java/com/iemr/tm/controller/cancerscreening/CancerScreeningController.java#L203-L203: use hasAnyRole('NURSE', 'DOCTOR').
• src/main/java/com/iemr/tm/controller/cancerscreening/CancerScreeningController.java#L233-L233: use hasAnyRole('NURSE', 'DOCTOR').
• src/main/java/com/iemr/tm/controller/cancerscreening/CancerScreeningController.java#L263-L263: use hasAnyRole('NURSE', 'DOCTOR').
• src/main/java/com/iemr/tm/controller/cancerscreening/CancerScreeningController.java#L295-L295: use hasAnyRole('NURSE', 'DOCTOR').
• src/main/java/com/iemr/tm/controller/cancerscreening/CancerScreeningController.java#L327-L327: use hasAnyRole('NURSE', 'DOCTOR').
• src/main/java/com/iemr/tm/controller/cancerscreening/CancerScreeningController.java#L359-L359: use hasAnyRole('NURSE', 'DOCTOR').
• src/main/java/com/iemr/tm/controller/cancerscreening/CancerScreeningController.java#L390-L390: use hasAnyRole('NURSE', 'DOCTOR').
• src/main/java/com/iemr/tm/controller/cancerscreening/CancerScreeningController.java#L417-L417: use hasAnyRole('NURSE', 'DOCTOR').
• src/main/java/com/iemr/tm/controller/cancerscreening/CancerScreeningController.java#L469-L469: use hasAnyRole('NURSE', 'DOCTOR').
• src/main/java/com/iemr/tm/controller/cancerscreening/CancerScreeningController.java#L508-L508: use hasAnyRole('NURSE', 'DOCTOR').
• src/main/java/com/iemr/tm/controller/cancerscreening/CancerScreeningController.java#L544-L544: use hasAnyRole('NURSE', 'DOCTOR', 'ONCOLOGIST').
• src/main/java/com/iemr/tm/controller/ncdscreening/NCDScreeningController.java#L143-L143: use hasAnyRole('NURSE', 'DOCTOR').
• src/main/java/com/iemr/tm/controller/ncdscreening/NCDScreeningController.java#L169-L169: use hasAnyRole('NURSE', 'DOCTOR').
• src/main/java/com/iemr/tm/controller/ncdscreening/NCDScreeningController.java#L198-L198: use hasAnyRole('NURSE', 'DOCTOR').
• src/main/java/com/iemr/tm/controller/ncdscreening/NCDScreeningController.java#L226-L226: use hasAnyRole('NURSE', 'DOCTOR').
• src/main/java/com/iemr/tm/controller/ncdscreening/NCDScreeningController.java#L253-L253: use hasAnyRole('NURSE', 'DOCTOR').
• src/main/java/com/iemr/tm/controller/ncdscreening/NCDScreeningController.java#L279-L279: use hasAnyRole('NURSE', 'DOCTOR').
• src/main/java/com/iemr/tm/controller/ncdscreening/NCDScreeningController.java#L306-L306: use hasAnyRole('NURSE', 'DOCTOR').
• src/main/java/com/iemr/tm/controller/ncdscreening/NCDScreeningController.java#L369-L369: use hasAnyRole('NURSE', 'DOCTOR').
• src/main/java/com/iemr/tm/controller/ncdscreening/NCDScreeningController.java#L398-L398: use hasAnyRole('NURSE', 'DOCTOR').
• src/main/java/com/iemr/tm/controller/ncdscreening/NCDScreeningController.java#L427-L427: use hasAnyRole('NURSE', 'DOCTOR').

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@src/main/java/com/iemr/tm/controller/cancerscreening/CancerScreeningController.java`
at line 143, Replace the chained hasRole authorization expressions on the
affected endpoints with hasAnyRole expressions. In CancerScreeningController at
lines 143, 173, 203, 233, 263, 295, 327, 359, 390, 417, 469, and 508, use NURSE
and DOCTOR; at line 544, include ONCOLOGIST as well. Apply the same NURSE/DOCTOR
hasAnyRole expression in NCDScreeningController at lines 143, 169, 198, 226,
253, 279, 306, 369, 398, and 427, preserving the existing access rules.

src/main/java/com/iemr/tm/service/health/HealthService.java (1)

90-90: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Consider externalizing ADVANCED_HEALTH_CHECKS_ENABLED.

It's hardcoded true; advanced checks add extra INFORMATION_SCHEMA queries per (throttled) health check. Exposing this via @Value with a sane default would allow disabling the extra DB load without a redeploy if it ever becomes a concern in production.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/main/java/com/iemr/tm/service/health/HealthService.java` at line 90,
Externalize ADVANCED_HEALTH_CHECKS_ENABLED instead of hardcoding it to true,
injecting it through the service’s configuration mechanism (such as `@Value`) with
a default of true. Preserve the existing advanced health-check behavior while
allowing operators to disable the extra database queries through configuration.

src/main/java/com/iemr/tm/service/generalOPD/GeneralOPDServiceImpl.java (1)

757-761: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Duplicated doctorSignatureFlag parsing logic.

The same 4-line null-safe boolean extraction is repeated in both saveDoctorData and updateGeneralOPDDoctorData. Consider extracting a small private helper (e.g. parseDoctorSignatureFlag(JsonObject requestOBJ)) to keep the two call sites in sync if the field name or semantics change.

Also applies to: 1368-1372

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/main/java/com/iemr/tm/service/generalOPD/GeneralOPDServiceImpl.java`
around lines 757 - 761, Extract the repeated null-safe doctorSignatureFlag
parsing from saveDoctorData and updateGeneralOPDDoctorData into a private helper
such as parseDoctorSignatureFlag(JsonObject requestOBJ). Replace both inline
extraction blocks with calls to the helper, preserving the current false default
and boolean conversion behavior.

src/main/java/com/iemr/tm/service/common/master/CommonMasterServiceImpl.java (1)

39-49: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Inject the interface instead of the concrete implementation class.

Injecting the concrete implementation (LabTechnicianServiceImpl) instead of its interface (LabTechnicianService) violates the Dependency Inversion Principle. It may also lead to BeanNotOfRequiredTypeException failures if Spring utilizes JDK dynamic proxies for transaction or security interception on the target bean.

Prefer injecting the LabTechnicianService interface.

♻️ Proposed refactor

-	private LabTechnicianServiceImpl labTechnicianServiceImpl;
+	private LabTechnicianService labTechnicianService;
 
 	`@Autowired`
 	public void setNcdCareMasterDataServiceImpl(NCDCareMasterDataServiceImpl ncdCareMasterDataServiceImpl) {
 		this.ncdCareMasterDataServiceImpl = ncdCareMasterDataServiceImpl;
 	}
 
 	`@Autowired`
-	public void setLabTechnicianServiceImpl(LabTechnicianServiceImpl labTechnicianServiceImpl) {
-		this.labTechnicianServiceImpl = labTechnicianServiceImpl;
+	public void setLabTechnicianService(LabTechnicianService labTechnicianService) {
+		this.labTechnicianService = labTechnicianService;
 	}

Make sure to update the usage on line 234:

 	`@Override`
 	public String getECGAbnormalFindings() {
-		return labTechnicianServiceImpl.getECGAbnormalFindings();
+		return labTechnicianService.getECGAbnormalFindings();
 	}

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/main/java/com/iemr/tm/service/common/master/CommonMasterServiceImpl.java`
around lines 39 - 49, Update CommonMasterServiceImpl to depend on the
LabTechnicianService interface instead of LabTechnicianServiceImpl: change the
field and setLabTechnicianServiceImpl setter parameter accordingly, while
preserving the existing setter wiring. Also update the usage around the
referenced call site to use the interface-typed dependency.

src/main/java/com/iemr/tm/utils/IntegerListConverter.java (1)

35-51: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low value

Cache the TypeToken type to prevent unnecessary object churn.

Using new TypeToken<List<Integer>>(){}.getType() recreates an anonymous inner class and executes reflection on every invocation of convertToEntityAttribute. Moving it to a static final constant is better for performance and memory allocation.

♻️ Proposed refactor

 import com.google.gson.Gson;
 import com.google.gson.reflect.TypeToken;
+import java.lang.reflect.Type;
 
 `@Converter`
 public class IntegerListConverter implements AttributeConverter<List<Integer>, String> {
 
     private final Gson gson = new Gson();
+    private static final Type LIST_TYPE = new TypeToken<List<Integer>>(){}.getType();
 
     `@Override`
     public String convertToDatabaseColumn(List<Integer> attribute) {
         if (attribute == null || attribute.isEmpty()) {
             return null;
         }
-        return gson.toJson(attribute);
+        return gson.toJson(attribute, LIST_TYPE);
     }
 
     `@Override`
     public List<Integer> convertToEntityAttribute(String dbData) {
         if (dbData == null || dbData.trim().isEmpty()) {
             return null;
         }
-        return gson.fromJson(dbData, new TypeToken<List<Integer>>(){}.getType());
+        return gson.fromJson(dbData, LIST_TYPE);
     }
 }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/main/java/com/iemr/tm/utils/IntegerListConverter.java` around lines 35 -
51, Cache the Gson type metadata used by IntegerListConverter in a static final
Type field, initialized once from the List<Integer> TypeToken. Update
convertToEntityAttribute to reuse this constant instead of creating a new
TypeToken on each invocation, while preserving the existing null and blank-input
behavior.

src/main/java/com/iemr/tm/service/registrar/RegistrarServiceImpl.java (1)

117-118: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Consider depending on RedisConnectionFactory instead of the concrete LettuceConnectionFactory.

Coding to the RedisConnectionFactory interface (which is all that's used here via getConnection()) improves testability and decouples this class from the specific Redis client implementation.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/main/java/com/iemr/tm/service/registrar/RegistrarServiceImpl.java` around
lines 117 - 118, Change the injected field in RegistrarServiceImpl from
LettuceConnectionFactory to the RedisConnectionFactory interface, retaining the
existing getConnection() usage and removing the concrete-client dependency.

src/main/java/com/iemr/tm/service/ncdCare/NCDCareServiceImpl.java (1)

774-778: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Fix indentation for readability.

The assignment statement inside the if block is missing an indentation level. Correcting the indentation ensures the code remains clean and visually clear.

• src/main/java/com/iemr/tm/service/ncdCare/NCDCareServiceImpl.java#L774-L778: indent doctorSignatureFlag = ... correctly.
• src/main/java/com/iemr/tm/service/ncdCare/NCDCareServiceImpl.java#L1211-L1215: indent doctorSignatureFlag = ... correctly.

🧹 Proposed fixes

For lines 774-778:

 			Boolean doctorSignatureFlag = false;
 			if (requestOBJ.has("doctorSignatureFlag") && !requestOBJ.get("doctorSignatureFlag").isJsonNull()) {
-			doctorSignatureFlag = requestOBJ.get("doctorSignatureFlag").getAsBoolean();
+				doctorSignatureFlag = requestOBJ.get("doctorSignatureFlag").getAsBoolean();
 			}

For lines 1211-1215:

 			Boolean doctorSignatureFlag = false;
 			if (requestOBJ.has("doctorSignatureFlag") && !requestOBJ.get("doctorSignatureFlag").isJsonNull()) {
-			doctorSignatureFlag = requestOBJ.get("doctorSignatureFlag").getAsBoolean();
+				doctorSignatureFlag = requestOBJ.get("doctorSignatureFlag").getAsBoolean();
 			}

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/main/java/com/iemr/tm/service/ncdCare/NCDCareServiceImpl.java` around
lines 774 - 778, Correct the indentation of the doctorSignatureFlag assignment
inside the if block at
src/main/java/com/iemr/tm/service/ncdCare/NCDCareServiceImpl.java lines 774-778
and lines 1211-1215, without changing its behavior.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

src/main/java/com/iemr/tm/controller/common/main/WorklistController.java (1)

710-731: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Add the same role guard to the three TC specialist worklist endpoints.
They only check authentication.isAuthenticated(), while the neighboring worklist handlers require a role. Add the same @PreAuthorize(...) restriction here so access control stays consistent.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/main/java/com/iemr/tm/controller/common/main/WorklistController.java`
around lines 710 - 731, Add the same `@PreAuthorize` role restriction used by
neighboring worklist handlers to getTCSpecialistWorkListNew and the other two TC
specialist worklist endpoint methods. Preserve their existing authentication and
response logic while applying the established role guard consistently to all
three endpoints.

src/main/java/com/iemr/tm/controller/health/HealthController.java (2)

40-43: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

git ls-files | rg 'src/main/java/.*/(HealthController|HealthService|SecurityConfig)\.java$'

Repository: PSMRI/TM-API

Length of output: 335

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '\n== files ==\n'
git ls-files | rg 'src/main/java/.*/(HealthController|HealthService|SecurityConfig)\.java$'

printf '\n== controller outline ==\n'
ast-grep outline src/main/java/com/iemr/tm/controller/health/HealthController.java --view expanded || true

printf '\n== security config outline ==\n'
ast-grep outline src/main/java/com/iemr/tm/config/SecurityConfig.java --view expanded || true

printf '\n== health service outline ==\n'
ast-grep outline src/main/java/com/iemr/tm/service/HealthService.java --view expanded || true

Repository: PSMRI/TM-API

Length of output: 828

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '\n== HealthController.java ==\n'
sed -n '1,220p' src/main/java/com/iemr/tm/controller/health/HealthController.java

printf '\n== HealthService.java ==\n'
sed -n '1,260p' src/main/java/com/iemr/tm/service/health/HealthService.java

printf '\n== SecurityConfig.java ==\n'
sed -n '1,260p' src/main/java/com/iemr/tm/utils/mapper/SecurityConfig.java

Repository: PSMRI/TM-API

Length of output: 17377

---

Keep the failure response shape consistent
HealthController.java:76-79 omits components, while the success path always returns it. Add the same top-level keys on the error path so callers don’t fail only when health checks break.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/main/java/com/iemr/tm/controller/health/HealthController.java` around
lines 40 - 43, Update the error response construction in HealthController to
include the same top-level keys as the successful health response, specifically
adding components alongside the existing status fields. Preserve the current
failure status and error details while ensuring both response paths share a
consistent shape.

---

73-82: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win

Error-path response omits components, unlike the success schema.

The catch-all fallback returns only {status, timestamp}, while the normal path always includes a components map. A client that unconditionally reads components.mysql.status will fail specifically when the health check itself throws — the scenario where a reliable response matters most.

🩹 Suggested fix

             Map<String, Object> errorResponse = Map.of(
                 "status", "DOWN",
-                "timestamp", Instant.now().toString()
+                "timestamp", Instant.now().toString(),
+                "components", Map.of()
             );

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

        } catch (Exception e) {
            logger.error("Unexpected error during health check", e);
            
            Map<String, Object> errorResponse = Map.of(
                "status", "DOWN",
                "timestamp", Instant.now().toString(),
                "components", Map.of()
            );
            
            return new ResponseEntity<>(errorResponse, HttpStatus.SERVICE_UNAVAILABLE);
        }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/main/java/com/iemr/tm/controller/health/HealthController.java` around
lines 73 - 82, Update the catch-all error response in HealthController to
include a components map matching the success schema, including the mysql status
entry, while preserving the existing DOWN status, timestamp, and
SERVICE_UNAVAILABLE response.

src/main/java/com/iemr/tm/service/benFlowStatus/CommonBenStatusFlowServiceImpl.java (1)

30-30: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Remove the unresolved and unused import.

This import appears to be an accidental inclusion (e.g., from an IDE auto-import). It is entirely unused within the class and will cause javac compilation failures if the checker-qual package is not available on the classpath.

🧹 Proposed fix

-import org.checkerframework.checker.units.qual.s;

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@src/main/java/com/iemr/tm/service/benFlowStatus/CommonBenStatusFlowServiceImpl.java`
at line 30, Remove the unused org.checkerframework.checker.units.qual.s import
from CommonBenStatusFlowServiceImpl, leaving the remaining imports and class
implementation unchanged.

src/main/java/com/iemr/tm/service/cancerScreening/CSServiceImpl.java (1)

814-819: 🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win

Add a null check for requestOBJ to prevent a potential NullPointerException.

The code unconditionally invokes requestOBJ.has(...), but the downstream logic (e.g., the null guard at line 820) indicates that requestOBJ can legitimately be null. If a null object is provided, this block will crash the request execution.

🛡️ Proposed fix to add the null guard and correct the indentation

 		Boolean doctorSignatureFlag = false;
-			if (requestOBJ.has("doctorSignatureFlag") && !requestOBJ.get("doctorSignatureFlag").isJsonNull()) {
-			doctorSignatureFlag = requestOBJ.get("doctorSignatureFlag").getAsBoolean();
-			}
+		if (requestOBJ != null && requestOBJ.has("doctorSignatureFlag") && !requestOBJ.get("doctorSignatureFlag").isJsonNull()) {
+			doctorSignatureFlag = requestOBJ.get("doctorSignatureFlag").getAsBoolean();
+		}

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

		Boolean doctorSignatureFlag = false;
		if (requestOBJ != null && requestOBJ.has("doctorSignatureFlag") && !requestOBJ.get("doctorSignatureFlag").isJsonNull()) {
			doctorSignatureFlag = requestOBJ.get("doctorSignatureFlag").getAsBoolean();
		}

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/main/java/com/iemr/tm/service/cancerScreening/CSServiceImpl.java` around
lines 814 - 819, Update the doctorSignatureFlag initialization block in
CSServiceImpl to guard requestOBJ before calling has or get; only read
doctorSignatureFlag when requestOBJ is non-null and the property is present and
non-null, preserving false as the default for a null requestOBJ.

src/main/java/com/iemr/tm/service/common/transaction/CommonServiceImpl.java (2)

566-595: 🔒 Security & Privacy | 🔴 Critical | ⚡ Quick win

Logging the raw OpenKM response can leak embedded credentials.

The method's own comment documents that the extracted URL can look like https://user:pass@https://host, meaning the response body legitimately contains Basic-Auth-style credentials embedded in a URL. logger.info("Response=" + response.getBody()) writes that entire body — credentials included — into application logs before any redaction happens.

🔒 Suggested fix

-				logger.info("Response=" + response.getBody());

 				String responseBody = response.getBody();

If response-body logging is needed for debugging, redact the credentials portion (e.g. strip anything before @) before logging, and drop it to DEBUG level.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

			logger.info("fileUUID for fileID " + obj.getInt("fileID") + " is " + fileUUID);
			logger.info("openkmDocUrl is " + openkmDocUrl);
			
			if (fileUUID != null) {
				Map<String, Object> requestBody = new HashMap<>();
				requestBody.put("fileUID", fileUUID);

				HttpEntity<Object> request = RestTemplateUtil.createRequestEntity(requestBody, Authorization);
				ResponseEntity<String> response = restTemplate.exchange(openkmDocUrl, HttpMethod.POST, request,
						String.class);

				String responseBody = response.getBody();
				if (responseBody != null) {
					JSONObject responseObj = new JSONObject(responseBody);
					if (responseObj.has("data")) {
						Object dataVal = responseObj.get("data");
						if (dataVal instanceof JSONObject) {
							JSONObject dataObj = (JSONObject) dataVal;
							if (dataObj.has("response")) {
								String fileUrl = dataObj.getString("response");
								// Fix malformed URL: https://user:pass@https://host -> https://user:pass@host
								fileUrl = fileUrl.replaceAll("`@https`?://", "@");
								return fileUrl;
							}
						}
						return dataVal.toString();
					}
				}
				return responseBody;

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/main/java/com/iemr/tm/service/common/transaction/CommonServiceImpl.java`
around lines 566 - 595, Update the response logging in the OpenKM exchange flow
before parsing responseBody: remove raw response-body logging at INFO level, or
log only a redacted version at DEBUG level by stripping embedded credentials
before “@”. Preserve the existing response parsing and URL normalization in the
surrounding fileUUID handling.

---

583-593: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Silent fallback returns a stringified JSON object as the "file URL".

When data is a JSONObject without a response key, dataVal.toString() is returned as if it were the download URL. The caller (WorklistController.getKMFile) puts this directly into the client response via response.setResponse(s), so the client would receive an unusable JSON blob instead of a URL, with no error signal.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/main/java/com/iemr/tm/service/common/transaction/CommonServiceImpl.java`
around lines 583 - 593, The file URL extraction logic in CommonServiceImpl must
not return dataVal.toString() when a JSONObject lacks the response key. Update
this fallback to signal that no usable URL was found, matching the existing
error or empty-result contract consumed by WorklistController.getKMFile, while
preserving response extraction and URL normalization for valid objects.

src/main/java/com/iemr/tm/utils/JwtAuthenticationUtil.java (1)

134-147: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Catch block re-wraps its own thrown IEMRException, losing message clarity and cause.

The throw new IEMRException("No role found...") at Line 141 is caught by the catch (Exception e) below it and re-wrapped into a confusing nested message ("Failed to retrieverole... error : No role found..."). The original exception's cause is also dropped since e isn't passed to the new IEMRException.

🐛 Proposed fix

 public List<String> getUserRoles(Long userId) throws IEMRException {
 		if (null == userId || userId <= 0) {
 			throw new IEMRException("Invalid User ID : " + userId);
 		}
 		try {
 			List<String> role = userLoginRepo.getRoleNamebyUserId(userId);
 			if (null == role || role.isEmpty()) {
 				throw new IEMRException("No role found for userId : " + userId);
 			}
 			return role;
+		} catch (IEMRException e) {
+			throw e;
 		} catch (Exception e) {
-			throw new IEMRException("Failed to retrieverole for usedId : " + userId + " error : " + e.getMessage());
+			throw new IEMRException("Failed to retrieve role for userId : " + userId + " error : " + e.getMessage(), e);
 		}
 	}

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

	public List<String> getUserRoles(Long userId) throws IEMRException {
		if (null == userId || userId <= 0) {
			throw new IEMRException("Invalid User ID : " + userId);
		}
		try {
			List<String> role = userLoginRepo.getRoleNamebyUserId(userId);
			if (null == role || role.isEmpty()) {
				throw new IEMRException("No role found for userId : " + userId);
			}
			return role;
		} catch (IEMRException e) {
			throw e;
		} catch (Exception e) {
			throw new IEMRException("Failed to retrieve role for userId : " + userId + " error : " + e.getMessage(), e);
		}
	}

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/main/java/com/iemr/tm/utils/JwtAuthenticationUtil.java` around lines 134
- 147, Update getUserRoles so the intentionally thrown IEMRException for a
missing role is not caught and re-wrapped by the general exception handler.
Preserve its original message, while continuing to wrap unexpected exceptions
with the original exception as the cause when constructing the failure
IEMRException.

src/main/java/com/iemr/tm/utils/JwtUserIdValidationFilter.java (1)

68-78: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win

Vary: Origin header dropped when echoing the allowed origin.

Previously the allowed-origin response set Vary: Origin; this block no longer sets it. Without it, caches/CDNs sitting in front of this service could serve one origin's CORS-enabled response to a different origin.

🛡️ Proposed fix

 			response.setHeader("Access-Control-Allow-Credentials", "true");
 			response.setHeader("Access-Control-Max-Age", "3600");
+			response.setHeader("Vary", "Origin");
 			logger.info("Origin Validated | Origin: {} | Method: {} | URI: {}", origin, method, uri);

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

		if (origin != null && isOriginAllowed(origin)) {
			response.setHeader("Access-Control-Allow-Origin", origin); // Never use wildcard
			response.setHeader("Access-Control-Allow-Methods", "GET, POST, PUT, PATCH, DELETE, OPTIONS");
			response.setHeader("Access-Control-Allow-Headers",
					"Authorization, Content-Type, Accept, Jwttoken, serverAuthorization, ServerAuthorization, serverauthorization, Serverauthorization");
			response.setHeader("Access-Control-Allow-Credentials", "true");
			response.setHeader("Access-Control-Max-Age", "3600");
			response.setHeader("Vary", "Origin");
			logger.info("Origin Validated | Origin: {} | Method: {} | URI: {}", origin, method, uri);
		} else {
			logger.warn("Origin [{}] is NOT allowed. CORS headers NOT added.", origin);
		}

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/main/java/com/iemr/tm/utils/JwtUserIdValidationFilter.java` around lines
68 - 78, Update the allowed-origin branch in JwtUserIdValidationFilter so it
restores the Vary response header with the value Origin alongside
Access-Control-Allow-Origin. Keep this header limited to responses where
isOriginAllowed(origin) succeeds.

src/main/java/com/iemr/tm/utils/mapper/RoleAuthenticationFilter.java (1)

150-165: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

resolveAuthToken doesn't strip the "Bearer " prefix, breaking the legacy Redis session lookup for standard clients.

HTTPRequestInterceptor.preHandle strips "Bearer " before using the token as a Redis key (preAuth.replace("Bearer ", "")). This method doesn't, so a client sending Authorization: Bearer <token> will have authToken = "Bearer <token>" passed to redisService.getObject(...), which won't find the session stored under the raw token — silently failing the legacy fallback path for standard Bearer-scheme clients.

🐛 Proposed fix

 private String resolveAuthToken(HttpServletRequest request) {

         String token = request.getHeader("Authorization");
+        if (token != null && token.startsWith("Bearer ")) {
+            token = token.substring(7);
+        }

         if (token == null || token.isBlank()) {
             token = request.getHeader("AuthToken");

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

    private String resolveAuthToken(HttpServletRequest request) {

        String token = request.getHeader("Authorization");
        if (token != null && token.startsWith("Bearer ")) {
            token = token.substring(7);
        }

        if (token == null || token.isBlank()) {
            token = request.getHeader("AuthToken");
        }
        if (token == null || token.isBlank()) {
            token = request.getHeader("X-Auth-Token");
        }
        if (token == null || token.isBlank()) {
            token = CookieUtil.getCookieValue(request, "Authorization")
                    .orElse(null);
        }
        return token;
    }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/main/java/com/iemr/tm/utils/mapper/RoleAuthenticationFilter.java` around
lines 150 - 165, Update resolveAuthToken in RoleAuthenticationFilter to remove
the leading “Bearer ” scheme prefix before returning the token, matching
HTTPRequestInterceptor.preHandle and preserving raw-token Redis session lookup.
Apply this normalization to the resolved Authorization token while leaving other
header and cookie fallback behavior unchanged.

src/main/java/com/iemr/tm/utils/mapper/SecurityConfig.java (1)

38-40: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

CSRF protection disabled while the app also authenticates via a cookie (Jwttoken).

CSRF is safe to disable only for stateless, header-only token auth with no browser-auto-submitted cookie. This codebase's JwtUserIdValidationFilter/CookieUtil treat the Jwttoken cookie as a first-class auth path, so state-changing requests authenticated via that cookie are not protected against forged cross-site requests with csrf().disable().

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/main/java/com/iemr/tm/utils/mapper/SecurityConfig.java` around lines 38 -
40, Update the SecurityConfig HTTP security chain to retain CSRF protection for
requests authenticated through the Jwttoken cookie, using the existing
JwtUserIdValidationFilter/CookieUtil authentication path as the reference. Do
not leave csrf globally disabled; configure an appropriate CSRF token repository
and cookie-based request handling while preserving stateless session management
and header-token behavior.

Source: Linters/SAST tools

src/main/java/com/iemr/tm/utils/redis/RedisStorage.java (1)

102-112: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

30-minute role cache creates a stale-privilege window after role changes/revocations.

cacheUserRoles sets a flat 30-minute TTL with no invalidation hook tied to role mutation elsewhere. Since RoleAuthenticationFilter treats this cache as authoritative for SecurityContextHolder authorities, a revoked or downgraded role can remain effectively granted for up to 30 minutes after the change.

Consider a shorter TTL and/or invalidating (redisTemplate.delete("roles:" + userId)) the cache entry whenever a user's role assignment changes, rather than relying solely on time-based expiry.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/main/java/com/iemr/tm/utils/redis/RedisStorage.java` around lines 102 -
112, Update cacheUserRoles and the role-assignment mutation flow to prevent
stale authorities: shorten the current 30-minute TTL and add explicit deletion
of the user’s "roles:" cache key whenever roles are assigned, revoked, or
changed. Ensure RoleAuthenticationFilter observes the updated roles after
mutation while preserving the existing cache write behavior.